Pin Power Pages Playwright MCP runtime - #383
Merged
Priyanshu Agrawal (priyanshu92) merged 19 commits intoAug 11, 2026
Merged
Conversation
- Keep runtime command approvals enabled by default - Explain credential scope and narrow script allow rules Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- pin the reviewed Playwright MCP runtime package to 0.0.78\n- invoke npx through Node with raw argv and no command shell\n- cover Windows paths, spawn failures, and exit propagation\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- require an absolute host-provided plugin root\n- canonicalize and contain the launcher path\n- cover invalid roots, malicious cwd, and supported hosts\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Priyanshu Agrawal (priyanshu92)
August 6, 2026 11:20
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Pins the Power Pages Playwright MCP bootstrap to a reviewed @playwright/mcp version and updates the launcher to invoke npx via Node (no shell) so config paths are passed as raw argv (including spaces/metacharacters), with improved error/exit propagation.
Changes:
- Pin runtime package selection to
@playwright/mcp@0.0.78and remove@latestusage. - Launch MCP via
node <npx-cli.js> ...withshell: false, preserving raw config paths. - Expand tests to validate version pinning, argv behavior, Windows
npx-cli.jsresolution, and spawn/exit failure handling.
Show a summary per file
| File | Description |
|---|---|
| plugins/power-pages/scripts/launch-playwright-mcp.js | Pins the MCP package version and changes process launch to node + npx-cli.js with shell: false, plus error/exit handling. |
| plugins/power-pages/scripts/tests/launch-playwright-mcp.test.js | Updates unit tests to assert exact version pinning, raw argv config paths, resolveNpxCli() behavior, and spawn/exit failure scenarios. |
| plugins/power-pages/scripts/tests/mcp-config.test.js | Adjusts bootstrap test strategy to intercept spawn and assert the pinned package/shell:false behavior during MCP server startup. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
…eadme-safety-guidance' into users/priyanshu92/fix-power-pages-mcp-root
Priyanshu Agrawal (priyanshu92)
changed the base branch from
main
to
users/priyanshu92/fix-power-pages-mcp-root
August 6, 2026 11:28
Copilot started reviewing on behalf of
Priyanshu Agrawal (priyanshu92)
August 6, 2026 11:29
View session
- stub only npx-cli existence probes in the MCP bootstrap preload\n- assert the deterministic npm-resolution seam is exercised\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Priyanshu Agrawal (priyanshu92)
August 6, 2026 11:50
View session
Contributor
There was a problem hiding this comment.
Review details
Suppressed comments (1)
plugins/power-pages/scripts/launch-playwright-mcp.js:65
launch()resolvesnpxCliPathvia a default parameter (npxCliPath = resolveNpxCli()), so if npm isn’t present /npx-cli.jscan’t be found, the error is thrown beforelaunch()can usewriteError/exitFn. That results in an uncaught exception + stack trace instead of the single-line failure output you added for spawn errors. Consider resolving inside the function body with try/catch and exiting cleanly (still allowing callers to passnpxCliPathexplicitly).
function launch({
browser = detectBrowser(),
npxCliPath = resolveNpxCli(),
spawnFn = spawn,
exitFn = (code) => process.exit(code),
writeError = (message) => process.stderr.write(message),
} = {}) {
const child = spawnFn(process.execPath, [npxCliPath, ...buildMcpArgs(browser)], {
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
- resolve the default npx CLI path inside launch error handling\n- report missing npm once without spawning\n- preserve explicit path injection and shell-free exact-pin behavior\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Priyanshu Agrawal (priyanshu92)
August 6, 2026 12:01
View session
T-Nid (tyaginidhi)
approved these changes
Aug 11, 2026
- Guard EACCES/EPERM errors while statting the declared plugin root. - Guard EACCES/EPERM errors while statting the resolved launcher. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7e799af6-1da3-4859-931b-4f199e3e1146
Preserve guarded root and launcher stat diagnostics while retaining the pinned, shell-free Playwright MCP launcher and deterministic npm-resolution seams. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Priyanshu Agrawal (priyanshu92)
August 11, 2026 07:24
View session
- Classify an exact-parent launcher resolution as escaping the declared plugin root. - Add deterministic coverage for the exact-parent containment boundary. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7e799af6-1da3-4859-931b-4f199e3e1146
Preserve the exact-parent containment boundary fix while retaining the pinned, shell-free Playwright MCP launcher and prior deterministic test seams. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Priyanshu Agrawal (priyanshu92)
August 11, 2026 07:30
View session
…iyanshu92/readme-safety-guidance
…iyanshu92/fix-power-pages-mcp-root
Inherit current main through the lower stack while preserving the exact Playwright MCP pin, shell-free launcher, npx error handling, and hardened root-resolution coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Priyanshu Agrawal (priyanshu92)
August 11, 2026 07:56
View session
Contributor
There was a problem hiding this comment.
Review details
Suppressed comments (1)
plugins/power-pages/scripts/launch-playwright-mcp.js:23
- This still fetches and executes
@playwright/mcpat runtime vianpx-cli.jswithout a committed lockfile or other integrity record, which conflicts with the plugin’s secure-coding requirement that runtime packages have an exact version and a committed lock / equivalent integrity boundary (and calls outscripts/launch-playwright-mcp.jsspecifically). Pinning the version is a good step, but consider switching to a vendored/checked-in artifact or adding a verifiable integrity mechanism (e.g., checked-in tarball + hash verification) so installs don’t execute unverified installer output during a plugin run.
// Marketplace installs copy only this plugin directory and do not run npm install,
// so a lockfile would not materialize a local executable. Keep the runtime package
// immutable, and disable lifecycle scripts while npx prepares the reviewed version.
return [
'--yes',
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
T-Nid (tyaginidhi)
approved these changes
Aug 11, 2026
- require an absolute host-provided plugin root\n- canonicalize and contain the launcher path\n- cover invalid roots, malicious cwd, and supported hosts\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- Guard EACCES/EPERM errors while statting the declared plugin root. - Guard EACCES/EPERM errors while statting the resolved launcher. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7e799af6-1da3-4859-931b-4f199e3e1146
- Classify an exact-parent launcher resolution as escaping the declared plugin root. - Add deterministic coverage for the exact-parent containment boundary. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7e799af6-1da3-4859-931b-4f199e3e1146
Adopt the rebased root-hardening history and current main while preserving the exact Playwright MCP pin, shell-free launcher, npx error handling, and deterministic review coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
gokulraj-g
approved these changes
Aug 11, 2026
Base automatically changed from
users/priyanshu92/fix-power-pages-mcp-root
to
main
August 11, 2026 09:39
Record the merged lower-layer history while retaining only the exact Playwright MCP pin, shell-free launcher, npx resolution/error behavior, and their tests as the main-relative PR delta. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Priyanshu Agrawal (priyanshu92)
deleted the
users/priyanshu92/pin-playwright-mcp
branch
August 11, 2026 09:45
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@playwright/mcppackage to reviewed version0.0.78npxentrypoint through Node with raw argv andshell: falseTesting
POWER_PLATFORM_SKILLS_TELEMETRY_POWER_PAGES_OPTOUT=1 node --test plugins/power-pages/scripts/tests/